Skip to content

Comments

refactor(atomic): migrate atomic-insight-search-box to Lit#6881

Merged
alexprudhomme merged 12 commits intomainfrom
copilot/migrate-atomic-insight-search-box
Jan 14, 2026
Merged

refactor(atomic): migrate atomic-insight-search-box to Lit#6881
alexprudhomme merged 12 commits intomainfrom
copilot/migrate-atomic-insight-search-box

Conversation

Copy link
Contributor

Copilot AI commented Jan 2, 2026

✅ Checklist

  • 🧪 The component is unit tested
  • 🧪 The component includes E2E tests
  • 🗑️ Old Cypress tests exclusive to the component are removed
  • 📖 The component is documented in storybook with an .mdx file
  • ♿ The component complies with the Web Content Accessibility Guidelines.
  • 🌐 All strings intended for humans or assistive technology must be localized with i18n.
  • 📦 The Lit component is exported in the appropriate index.ts and lazy-index.ts files.
  • 🎨 CSS parts are documented still accessible.
  • 🦥 Slotted Content, public methods and properties are documented
  • 🔄 The component outputs the same Angular output as before with Stencil
  • 🏷️ The component declares the component type in the HTMLElementTagNameMap

https://coveord.atlassian.net/browse/KIT-4933

Copilot AI self-assigned this Jan 2, 2026
Copilot AI and others added 2 commits January 2, 2026 13:52
…ncil to Lit

Co-authored-by: alexprudhomme <78121423+alexprudhomme@users.noreply.github.com>
…h-box

Co-authored-by: alexprudhomme <78121423+alexprudhomme@users.noreply.github.com>
Copilot AI changed the title [WIP] Migrate the atomic-insight-search-box component to Lit feat(atomic): migrate atomic-insight-search-box to Lit Jan 2, 2026
Copilot AI requested a review from alexprudhomme January 2, 2026 14:01
@alexprudhomme alexprudhomme changed the title feat(atomic): migrate atomic-insight-search-box to Lit refactor(atomic): migrate atomic-insight-search-box to Lit Jan 2, 2026
Copy link
Contributor

@SimonMilord SimonMilord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@alexprudhomme alexprudhomme added this pull request to the merge queue Jan 14, 2026
Merged via the queue into main with commit ca2020a Jan 14, 2026
182 of 185 checks passed
@alexprudhomme alexprudhomme deleted the copilot/migrate-atomic-insight-search-box branch January 14, 2026 20:49
Copy link
Collaborator

@erocheleau erocheleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this was already merged, but I do have some comments and questions as there are changes between the stencil version and this one.

expect(icon).not.toBeInTheDocument();
});

it('should render the icon when alwaysShowIcon is true even if hasMultipleKindOfSuggestions is false', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I'm clear on the relation between hasMultipleKindOfSuggestions and the option alwaysShowIcon ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alwaysShowIcon is an override that forces the icon to display even when hasMultipleKindOfSuggestions is false (which normally hides icons when there's only one suggestion type).

* When true, the icon will always be displayed regardless of hasMultipleKindOfSuggestions.
* Used by the insight search box where icons should always be visible.
*/
alwaysShowIcon?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's that exactly?
The icon next to the query suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes exactly. For search we dont always show it

const {textArea} = await renderComponent();
await userEvent.type(textArea, 'test');
expect(dispatchSpy).toHaveBeenCalledWith(
expect.objectContaining({type: 'fetchQuerySuggestions'})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also validate here that it was called with the right query value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we could do that.

Comment on lines +253 to +254
hasMultipleKindOfSuggestions: false,
alwaysShowIcon: this.searchBoxState.suggestions.length > 1,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still unclear on the relation between these two attributes that seem to decide if we're supposed to render an icon or not?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When suggestions are there, we want to always show an icon for insight. Search has logic for this, insight just always shows an icon so I added this prop to make it work.

getRef: () => HTMLElement | undefined
): TemplateResult | typeof nothing {
if (!elements.length) {
return nothing;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again with the nothing instead of null?
Is it expected that the lit nothing is not the same type as TemplateResult ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is expected and quite annoying imo, we had to add it in some internal types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants